-
Notifications
You must be signed in to change notification settings - Fork 86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CIR] Cleanup cir.scopes with a single cir.yield operation #482
Conversation
This diverges from the original codegen by tracking all members of a union, instead of just the largest one. This is necessary to support type-checking at the MLIR level when accessing union members. It also preserves more information about the source code, which might be useful. Fixes llvm#224 ghstack-source-id: 8a975426d077a66c49f050741d7362da3c102fed Pull Request resolved: llvm#229
Converts a union to a struct containing only its largest element. GetMemberOp for unions is lowered as bitcasts instead of GEPs, since union members share the same address space. ghstack-source-id: 744ac312675b8f3225ccc459fcd09474bcfcfe81 Pull Request resolved: llvm#230
…lvm#241) Similar with the previous ctor support, I'm bringing up the dtor support for global var initialization. This change only contains the CIR early codegen work. The upcoming lowering prepare work will be in a separate change.
This change adds lowering support for global variable definition with destructor. For example: ``` cir.global "private" internal @_ZL8__ioinit = ctor : !ty_22Init22 { %0 = cir.get_global @_ZL8__ioinit : cir.ptr <!ty_22Init22> %1 = cir.const(#true) : !cir.bool cir.call @_ZN4InitC1Eb(%0, %1) : (!cir.ptr<!ty_22Init22>, !cir.bool) -> () } dtor { %0 = cir.get_global @_ZL8__ioinit : cir.ptr <!ty_22Init22> cir.call @_ZN4InitD1Ev(%0) : (!cir.ptr<!ty_22Init22>) -> () } ``` is now lowered to ``` cir.func internal private @__cxx_global_var_init() { %0 = cir.get_global @_ZL8__ioinit : cir.ptr <!ty_22Init22> %1 = cir.const(#true) : !cir.bool cir.call @_ZN4InitC1Eb(%0, %1) : (!cir.ptr<!ty_22Init22>, !cir.bool) -> () %2 = cir.get_global @_ZL8__ioinit : cir.ptr <!ty_22Init22> %3 = cir.get_global @_ZN4InitD1Ev : cir.ptr <!cir.func<!void (!cir.ptr<!ty_22Init22>)>> %4 = cir.cast(bitcast, %3 : !cir.ptr<!cir.func<!void (!cir.ptr<!ty_22Init22>)>>), !cir.ptr<!cir.func<!void (!cir.ptr<!void>)>> %5 = cir.cast(bitcast, %2 : !cir.ptr<!ty_22Init22>), !cir.ptr<!void> %6 = cir.get_global @__dso_handle : cir.ptr <i8> cir.call @__cxa_atexit(%4, %5, %6) : (!cir.ptr<!cir.func<!void (!cir.ptr<!void>)>>, !cir.ptr<!void>, !cir.ptr<i8>) -> () cir.return } ``` Note that instead calling the destructor in the global initializer function, a registration with `__cxa_atexit` is done instead so that the destructor will be called right before the program is shut down.
Add two new CIR ops: cir.alloc_exception and cir.throw, they are higher level representations for their __cxa_* counterparts. Add CIRGen for a simple `throw <expr>` example, which requires using the above added operations. Rethrow mechanism (plain `throw`) is still NYI. For LLVM lowering: LoweringPrepare work needs to be done next, breaking these ops back to functions calls.
Add an assert while we don't handle the available externally.
Add lots of necessary boilerplate for personality functions, catch block tracking and building catch scope stack. We enter the try but no catching handling yet. New paths all guarded with asserts, therefore NFC.
In order to support exceptions, skeleton for unwind detection.
…call attributes soon
Populate regions and basic blocks for handling try-catch! No testcases just yet, new path guarded with unreachable, so effectively this is NFC.
This PR removes the method `hasBooleanRepresentation` as was discussed in [PR#233](llvm#233) Briefly, the `cir.bool` has the same representation in memory and after load. The LLVM IR differs, that's why the check is there, in the origin `CodeGen`. Also, in order to trigger the path and make the implementation to be conform with the original `CodeGen`, there are changes in the `CIRGenExprScalar`: call the common `buildFromLValue` instead of manaul `load` creation. Also, a couple of tests for the bool load/store added
…n. (llvm#253) Introducing `cir.ConstPtrAttr` to represent arbitrary absolute pointer value initializations. Also incorporating previous `cir.nullptr` effort into this work.
Following discussion in llvm#237 this adds support for `fabs` builtins which are used extensively in llvm-test-suite.
Temporary workaround until we patch the codegen for record types.
This patch brings up the basic support for C++ virtual inheritance. VTT (virtual table table) now can be laid out as expected for simple program with single virtual inheritance. RTTI support is on the way. This patch does not include LLVM lowering support.
This PR introduces bitfelds support. This now works: ``` #include <stdio.h> typedef struct { int a1 : 4; int a2 : 28; int a3 : 16; int a4 : 3; int a5 : 17; int a6 : 25; } A; void init(A* a) { a->a1 = 1; a->a2 = 321; a->a3 = 15; a->a4 = -2; a->a5 = -123; a->a6 = 1234; } void print(A* a) { printf("%d %d %d %d %d %d\n", a->a1, a->a2, a->a3, a->a4, a->a5, a->a6 ); } int main() { A a; init(&a); print(&a); return 0; } ``` the output is: `1 321 15 -2 -123 1234`
- Introduces `CIR/Interfaces/ASTAttrInterfaces` which model API of clang AST nodes, but allows to plugin custom attribute, making `CIR` dialect AST independent. - Extends hierarchy of `DeclAttr`s to model `Decl` attributes more faithfully. - Notably all `CIRASTAttr`s are now created uniformly using `makeAstDeclAttr` which builds corresponding Attribute based on `clang::Decl`.
…lvm#263) There is a bug in the code generation: the field index for the `GetMemberOp` is taken from the `FieldDecl`, with no respect to the record layout. One of the manifestation of the bug is the wrong index generated for a field in a derived class that does not take into the account the instance of the base class (that has index 0). You can take a look at the example in `test/CIR/CodeGen/derived-to-base.cpp`, i.e. the current test is not the correct one ``` // CHECK-DAG: !ty_22C23A3ALayer22 = !cir.struct<class "C2::Layer" {!ty_22C13A3ALayer22, !cir.ptr<!ty_22C222> // CHECK-DAG: !ty_22C33A3ALayer22 = !cir.struct<struct "C3::Layer" {!ty_22C23A3ALayer22 // CHECK: cir.func @_ZN2C35Layer10InitializeEv // CHECK: cir.scope { // CHECK: %2 = cir.base_class_addr(%1 : cir.ptr <!ty_22C33A3ALayer22>) -> cir.ptr <!ty_22C23A3ALayer22> // CHECK: %3 = cir.get_member %2[0] {name = "m_C1"} : !cir.ptr<!ty_22C23A3ALayer22> -> !cir.ptr<!cir.ptr<!ty_22C222>> ``` As one can see, the result (of ptr type to `!ty_22C222` ) must have the index `1` in the corresponded struct `ty_22C23A3ALayer22`. Basically the same is done in the `clang/CodeGen/CGExpr.cpp`, so we don't invent something new here. Note, this fix doesn't affect anything related to the usage of `buildPreserveStructAccess` where the `field->getFieldIndex()` is used.
This PR adds MLIR lowering of `cir.scope`. I also notice that the MLIR unit tests still uses old integer types. I will fix those in a separate PR.
Recent work on exceptions broke an internal testcase, free the path back while I work on a holistic solution.
This PR adds missing case to lowerCirAttrAsValue.
…r.try The final destination here is to support cir.try_calls that are not within a `try {}` statement in C++. This only affect untested paths that will assert a bit later than before, testcase coming soon.
The second part of the job started in llvm#412 , now about local structures. As it was mentioned previously, sometimes the layout for structures with bit fields inited with constants differ from the originally created in `CIRRecordLayoutBuilder` and it cause `storeOp` verification fail due to different structure type was used to allocation. This PR fix it. An example: ``` typedef struct { int a : 4; int b : 5; int c; } D; void bar () { D d = {1,2,3}; } ``` Well, I can't say I'm proud of these changes - it seems like a type safety violation, but looks like it's the best we can do here. The original codegen doesn't have this problem at all, there is just a `memcpy` there, I provide LLVM IR just for reference: ``` %struct.D = type { i16, i32 } @__const.bar.d = private unnamed_addr constant { i8, i8, i32 } { i8 33, i8 0, i32 3 }, align 4 ; Function Attrs: noinline nounwind optnone uwtable define dso_local void @bar() #0 { entry: %d = alloca %struct.D, align 4 call void @llvm.memcpy.p0.p0.i64(ptr align 4 %d, ptr align 4 @__const.bar.d, i64 8, i1 false) ret void } ```
In llvm#426 we confirmed that CIR needs a `cir.unreachable` operation to mark unreachable program points [(discussion)](llvm#426 (comment)). This PR adds it.
This PR adds support for evaluating constants in member exprs. The change is taken from original codegen.
- Add it to functions but not yet on calls. - Add more skeleton for tagging function attributes. - Testcases One more incremental step towards cir.try_call outside cir.try scopes.
The next step in inline-assembly support: we add instruction operands! Nothing interesting, just some copy-pasta from the `codegen` with some sort of simplifications for now. Well, I'm not sure `functional-type` is the best way to print operands though it's used in mlir's `InlineAsmOp`. But anyways, maybe you have a better idea. There are two or three steps ahead, so we are not that far from being able to run something!
One more tiny step! This a tiny PR that adds clobbers to constraint string. Note, that `~{dirflag},~{fpsr},~{flags}` is a [X86](https://github.com/llvm/clangir/blob/main/clang/lib/Basic/Targets/X86.h#L281) dependent clobbers. Basically, the next things remain: - lowering - store the results of the `cir.asm`
You can test this locally with the following command:git-clang-format --diff b9cd201198ee61b57cfbdfced69c9fe2f3710cf5 a984329e4fcbc220ec681e6ed1b9ee2aad20c487 -- clang/lib/CIR/Dialect/Transforms/MergeCleanups.cpp View the diff from clang-format here.diff --git a/clang/lib/CIR/Dialect/Transforms/MergeCleanups.cpp b/clang/lib/CIR/Dialect/Transforms/MergeCleanups.cpp
index b12f4c9f08..42dcb6e965 100644
--- a/clang/lib/CIR/Dialect/Transforms/MergeCleanups.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/MergeCleanups.cpp
@@ -59,10 +59,10 @@ struct RemoveEmptyScope : public OpRewritePattern<ScopeOp> {
LogicalResult match(ScopeOp op) const final {
return success(op.getRegion().empty() ||
- (op.getRegion().getBlocks().size() == 1 &&
- op.getRegion().front().empty()) ||
- (op.getRegion().front().getOperations().size() == 1 &&
- isa<YieldOp>(&op.getRegion().front().front())));
+ (op.getRegion().getBlocks().size() == 1 &&
+ op.getRegion().front().empty()) ||
+ (op.getRegion().front().getOperations().size() == 1 &&
+ isa<YieldOp>(&op.getRegion().front().front())));
}
void rewrite(ScopeOp op, PatternRewriter &rewriter) const final {
|
Thanks, this also needs a testcase! |
Thanks for this comment. Just to clarify, I should add the test case to clangir/tree/main/clang/test/CIR/Transforms/merge-cleanups.cir, right? |
That'd be a good place for it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some NIT
op.getRegion().front().empty())); | ||
(op.getRegion().getBlocks().size() == 1 && | ||
op.getRegion().front().empty()) || | ||
(std::distance(op.getRegion().front().begin(), op.getRegion().front().end()) == 1 && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To test a block has only one operation, you can use:
op.getRegion().front().getOperations().size() == 1
Which is clearer.
Expected behavior is that scope blocks with only a single yield op are cleaned up.
ed3955a
to
8b7417c
Compare
Any plans to continue the work here? |
c4db6d0
to
e197d4e
Compare
Gonna close this for now in order to keep tracking of only active PRs, feel free to re-open if there are updated. |
This PR adds an extra condition to the match pattern in the RemoveEmptyScope cleanup pass to consider the case where there is a single operation in the block and that it is YieldOp.
Issue #455
Related to #436